Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix HTTP proxying #1415

Closed
wants to merge 2 commits into from
Closed

fix HTTP proxying #1415

wants to merge 2 commits into from

Conversation

pawelmhm
Copy link
Contributor

@pawelmhm pawelmhm commented Nov 20, 2016 •
edited
Loading

What do these changes do?

  • fixes HTTP proxy support.
  • adds functional tests for proxies (via Mitmdump)

Are there changes in behavior for the user?

Fixes regression, brings back intended behavior, users will be able to use HTTP proxies without any errors.

Related issue number

#1413

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new entry to CHANGES.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using #issue_number format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.

@pawelmhm
fix HTTP proxying
ac8b43f 
* fix bug that broke HTTP proxy support
* add functional tests via mitmdump

fixes aio-libs#1413
@pawelmhm
disable functional proxy tests for python 3.4 & windows
224e731 
mitmdump doesn't support python 3.4, I can't test on Windows.
@codecov-io
Copy link

Current coverage is 98.50% (diff: 100%)

Merging #1415 into master will decrease coverage by 0.37%

@@             master      #1415   diff @@
==========================================
  Files            30         30          
  Lines          6900       6902     +2   
  Methods           0          0          
  Messages          0          0          
  Branches       1143       1144     +1   
==========================================
- Hits           6823       6799    -24   
- Misses           33         39     +6   
- Partials         44         64    +20

Powered by Codecov. Last update b65ecf4...ac8b43f

@nodermann2
Copy link

nodermann2 commented Nov 21, 2016 •
edited
Loading

Long story short

My apologies if not posted there, I tried this patch and found the related bug, it appears after a redirect, I think I should put it here until it's in production

Expected behaviour

http://httpbin.org/redirect/6 (302) -> http://httpbin.org/get (200)

Actual behaviour

Traceback (most recent call last):
  File "/home/user/Projects/test_beh.py", line 14, in <module>
    loop.run_until_complete(go())
  File "/usr/lib/python3.5/asyncio/base_events.py", line 387, in run_until_complete
    return future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 274, in result
    raise self._exception
  File "/usr/lib/python3.5/asyncio/tasks.py", line 239, in _step
    result = coro.send(None)
  File "/home/user/Projects/test_beh.py", line 8, in go
    proxy="http://23.81.247.47:29842") as resp:
  File "/home/user/envs/test_env/lib/python3.5/site-packages/aiohttp/client.py", line 529, in __aenter__
    self._resp = yield from self._coro
  File "/home/user/envs/test_env/lib/python3.5/site-packages/aiohttp/client.py", line 168, in _request
    resp = req.send(conn.writer, conn.reader)
  File "/home/user/envs/test_env/lib/python3.5/site-packages/aiohttp/client_reqrep.py", line 442, in send
    path = self.path
AttributeError: 'ClientRequest' object has no attribute 'path'

Steps to reproduce

import asyncio
import aiohttp

async def go():
    async with aiohttp.ClientSession() as session:
        async with session.get("http://httpbin.org/redirect/6",
                               proxy="http://163.172.177.100:80") as resp:
            print(resp.status)


loop = asyncio.get_event_loop()
loop.run_until_complete(go())
loop.close()

Your environment

python 3.5.2
aiohttp 1.1.5 with this patch

Other

random proxy list http://hideme.ru/proxy-list/?ports=80#list

@asvetlov
Copy link
Member

There is applied #1421
Please check with a fresh master.
I have nothing against mitmproxy (or other real proxy server, maybe https://github.com/hawkowl/rproxy is also a good candidate for running twisted based server in the same loop -- I don't know).

@asvetlov
Copy link
Member

I believe it should be closed

@asvetlov asvetlov closed this Nov 26, 2016
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
outdated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pawelmhm @codecov-io @nodermann2 @asvetlov